-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store Testing Framework Solidity and Yul #1052
Conversation
48c7e3f
to
e6ca2b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. However, I think some documentation would be useful to clarify what exactly is going on (e.g. around SolidityUtils.getContractByteCode()
.
private static final ObjectMapper objectMapper = new ObjectMapper(); | ||
private static final ClassLoader classLoader = SolidityUtils.class.getClassLoader(); | ||
|
||
public static Bytes getContractByteCode(Class<? extends Contract> contractClass) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not super clear on what's happening here. I think it would be helpful to write some amount of documentation to explain (at a high level) what's going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some documentation for this method.
arithmetization/src/test/java/net/consensys/linea/zktracer/ExampleSolidityTest.java
Outdated
Show resolved
Hide resolved
arithmetization/src/test/java/net/consensys/linea/zktracer/ExampleSolidityTest.java
Outdated
Show resolved
Hide resolved
arithmetization/src/test/java/net/consensys/linea/zktracer/ExampleSolidityTest.java
Outdated
Show resolved
Hide resolved
import org.web3j.tx.Contract; | ||
|
||
public class SolidityUtils { | ||
private static final ObjectMapper objectMapper = new ObjectMapper(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend using here JsonConverter
class the embeds the ObjectMapper
and offers some helpful functionalities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API for JsonConverter
requires the Class that can parsed from the json file. I do not have a Class for that, I am interested in just one particular attribute 'bin-runtime' of a specific contract in the json file.
#1139